Skip to content

test default PassField impl handles output parameters #221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 2, 2025
Merged

test default PassField impl handles output parameters #221

merged 2 commits into from
Oct 2, 2025

Conversation

Eunovo
Copy link
Contributor

@Eunovo Eunovo commented Oct 1, 2025 •
edited
Loading

This PR adds two test cases to ensure that output parameter results from a server call are correctly captured and included in the response.
The new tests also serve as practical examples of how to work with output parameters and in|out parameters in the codebase.

Testing

Automated coverage is not yet available, see #198.

As a temporary verification method:

  • Comment out these lines
    and run make check. The new test should fail, confirming it is effective.

  • If you then also comment out the newly added tests and rerun make check, it should pass, demonstrating that the lines above were previously uncovered.

@DrahtBot
Copy link

DrahtBot commented Oct 1, 2025 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK ryanofsky

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@ryanofsky ryanofsky mentioned this pull request Oct 1, 2025
Merged
ryanofsky
ryanofsky reviewed Oct 1, 2025
View reviewed changes
Copy link
Collaborator

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review f3688fd. Nice test! I'm surprised there wasn't already a test for output parameters. Would suggest making the simplification below but otherwise this looks good.

Eunovo added 2 commits October 1, 2025 21:58
@Eunovo
test default PassField impl handles output parameters
29cf2ad 
Tests that output parameter result is captured from server call and added to response.
@Eunovo
test In|Out parameter
6db6696
@Eunovo Eunovo force-pushed the test-return-parameter branch from f3688fd to 6db6696 Compare October 1, 2025 20:02
ryanofsky
ryanofsky approved these changes Oct 1, 2025
View reviewed changes
Copy link
Collaborator

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK 6db6696. Thanks, both changes look good!

@ryanofsky ryanofsky merged commit 58cf47a into bitcoin-core:master Oct 2, 2025
8 checks passed
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Oct 2, 2025
@ryanofsky
Squashed 'src/ipc/libmultiprocess/' changes from 47d79db8a552..ec86e4…
3f74b5a 
…336e98

ec86e4336e98 Merge bitcoin-core/libmultiprocess#220: Add log levels and advertise them to users via logging callback
515ce93ad349 Logging: Pass LogData struct to logging callback
213574ccc43d Logging: reclassify remaining log messages
e4de0412b430 Logging: Break out expensive log messages and classify them as Trace
408874a78fdc Logging: Use new logging macros
67b092d835cd Logging: Disable logging if messsage level is less than the requested level
d0a1ba7ebf21 Logging: add log levels to mirror Core's
463a8296d188 Logging: Disable moving or copying Logger
83a2e10c0b03 Logging: Add an EventLoop constructor to allow for user-specified log options
58cf47a7fc8c Merge bitcoin-core/libmultiprocess#221: test default PassField impl handles output parameters
db03a663f514 Merge bitcoin-core/libmultiprocess#214: Fix crash on simultaneous IPC calls using the same thread
afcc40b0f1e8 Merge bitcoin-core/libmultiprocess#213: util+doc: Clearer errors when attempting to run examples + polished docs
6db669628387 test In|Out parameter
29cf2ada75ea test default PassField impl handles output parameters
1238170f68e8 test: simultaneous IPC calls using same thread
eb069ab75d83 Fix crash on simultaneous IPC calls using the same thread
ec03a9639ab5 doc: Precision and typos
2b4348193551 doc: Where possible, remove links to ryanofsky/bitcoin/
286fe469c9c9 util: Add helpful error message when failing to execute file

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: ec86e4336e986a02b08ab12f7eea9f74551c5bef
@ryanofsky ryanofsky mentioned this pull request Oct 2, 2025
Open
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Oct 2, 2025
@ryanofsky
Squashed 'src/ipc/libmultiprocess/' changes from 13424cf2ecc1..ec86e4…
5bb3cbd 
…336e98

ec86e4336e98 Merge bitcoin-core/libmultiprocess#220: Add log levels and advertise them to users via logging callback
515ce93ad349 Logging: Pass LogData struct to logging callback
213574ccc43d Logging: reclassify remaining log messages
e4de0412b430 Logging: Break out expensive log messages and classify them as Trace
408874a78fdc Logging: Use new logging macros
67b092d835cd Logging: Disable logging if messsage level is less than the requested level
d0a1ba7ebf21 Logging: add log levels to mirror Core's
463a8296d188 Logging: Disable moving or copying Logger
83a2e10c0b03 Logging: Add an EventLoop constructor to allow for user-specified log options
58cf47a7fc8c Merge bitcoin-core/libmultiprocess#221: test default PassField impl handles output parameters
db03a663f514 Merge bitcoin-core/libmultiprocess#214: Fix crash on simultaneous IPC calls using the same thread
afcc40b0f1e8 Merge bitcoin-core/libmultiprocess#213: util+doc: Clearer errors when attempting to run examples + polished docs
6db669628387 test In|Out parameter
29cf2ada75ea test default PassField impl handles output parameters
1238170f68e8 test: simultaneous IPC calls using same thread
eb069ab75d83 Fix crash on simultaneous IPC calls using the same thread
ec03a9639ab5 doc: Precision and typos
2b4348193551 doc: Where possible, remove links to ryanofsky/bitcoin/
286fe469c9c9 util: Add helpful error message when failing to execute file
47d79db8a552 Merge bitcoin-core/libmultiprocess#201: bug: fix mptest hang, ProxyClient<Thread> deadlock in disconnect handler
f15ae9c9b9fb Merge bitcoin-core/libmultiprocess#211: Add .gitignore
4a269b21b8c8 bug: fix ProxyClient<Thread> deadlock if disconnected as IPC call is returning
85df96482c49 Use try_emplace in SetThread instead of threads.find
ca9b380ea91a Use std::optional in ConnThreads to allow shortening locks
9b0799113557 doc: describe ThreadContext struct and synchronization requirements
d60db601ed9b proxy-io.h: add Waiter::m_mutex thread safety annotations
4e365b019a9f ci: Use -Wthread-safety not -Wthread-safety-analysis
15d7bafbb001 Add .gitignore
fe1cd8c76131 Merge bitcoin-core/libmultiprocess#208: ci: Test minimum cmake version in olddeps job
b713a0b7bfbc Merge bitcoin-core/libmultiprocess#207: ci: output CMake version in CI script
0f580397c913 ci: Test minimum cmake version in olddeps job
d603dcc0eef0 ci: output CMake version in CI script

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: ec86e4336e986a02b08ab12f7eea9f74551c5bef
@ryanofsky ryanofsky mentioned this pull request Oct 2, 2025
Open
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Oct 7, 2025
@ryanofsky
Squashed 'src/ipc/libmultiprocess/' changes from 47d79db8a552..f4344a…
5191781 
…e87da0

f4344ae87da0 Merge bitcoin-core/libmultiprocess#222: test, ci: Fix threadsanitizer errors in mptest
73d22ba2e930 test: Fix tsan race in thread busy test
b74e1bba014d ci: Use tsan-instrumented cap'n proto in sanitizers job
c332774409ad test: Fix failing exception check in new thread busy test
ca3c05d56709 test: Use KJ_LOG instead of std::cout for logging
7eb1da120ab6 ci: Use tsan-instrumented libcxx in sanitizers job
ec86e4336e98 Merge bitcoin-core/libmultiprocess#220: Add log levels and advertise them to users via logging callback
515ce93ad349 Logging: Pass LogData struct to logging callback
213574ccc43d Logging: reclassify remaining log messages
e4de0412b430 Logging: Break out expensive log messages and classify them as Trace
408874a78fdc Logging: Use new logging macros
67b092d835cd Logging: Disable logging if messsage level is less than the requested level
d0a1ba7ebf21 Logging: add log levels to mirror Core's
463a8296d188 Logging: Disable moving or copying Logger
83a2e10c0b03 Logging: Add an EventLoop constructor to allow for user-specified log options
58cf47a7fc8c Merge bitcoin-core/libmultiprocess#221: test default PassField impl handles output parameters
db03a663f514 Merge bitcoin-core/libmultiprocess#214: Fix crash on simultaneous IPC calls using the same thread
afcc40b0f1e8 Merge bitcoin-core/libmultiprocess#213: util+doc: Clearer errors when attempting to run examples + polished docs
6db669628387 test In|Out parameter
29cf2ada75ea test default PassField impl handles output parameters
1238170f68e8 test: simultaneous IPC calls using same thread
eb069ab75d83 Fix crash on simultaneous IPC calls using the same thread
ec03a9639ab5 doc: Precision and typos
2b4348193551 doc: Where possible, remove links to ryanofsky/bitcoin/
286fe469c9c9 util: Add helpful error message when failing to execute file

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: f4344ae87da0375ca32b9967b3c0ac381a3349d2
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Oct 7, 2025
@ryanofsky
Squashed 'src/ipc/libmultiprocess/' changes from 47d79db8a552..a4f929…
0f01e15 
…696490

a4f929696490 Merge bitcoin-core/libmultiprocess#224: doc: fix typos
f4344ae87da0 Merge bitcoin-core/libmultiprocess#222: test, ci: Fix threadsanitizer errors in mptest
1434642b3804 doc: fix typos
73d22ba2e930 test: Fix tsan race in thread busy test
b74e1bba014d ci: Use tsan-instrumented cap'n proto in sanitizers job
c332774409ad test: Fix failing exception check in new thread busy test
ca3c05d56709 test: Use KJ_LOG instead of std::cout for logging
7eb1da120ab6 ci: Use tsan-instrumented libcxx in sanitizers job
ec86e4336e98 Merge bitcoin-core/libmultiprocess#220: Add log levels and advertise them to users via logging callback
515ce93ad349 Logging: Pass LogData struct to logging callback
213574ccc43d Logging: reclassify remaining log messages
e4de0412b430 Logging: Break out expensive log messages and classify them as Trace
408874a78fdc Logging: Use new logging macros
67b092d835cd Logging: Disable logging if messsage level is less than the requested level
d0a1ba7ebf21 Logging: add log levels to mirror Core's
463a8296d188 Logging: Disable moving or copying Logger
83a2e10c0b03 Logging: Add an EventLoop constructor to allow for user-specified log options
58cf47a7fc8c Merge bitcoin-core/libmultiprocess#221: test default PassField impl handles output parameters
db03a663f514 Merge bitcoin-core/libmultiprocess#214: Fix crash on simultaneous IPC calls using the same thread
afcc40b0f1e8 Merge bitcoin-core/libmultiprocess#213: util+doc: Clearer errors when attempting to run examples + polished docs
6db669628387 test In|Out parameter
29cf2ada75ea test default PassField impl handles output parameters
1238170f68e8 test: simultaneous IPC calls using same thread
eb069ab75d83 Fix crash on simultaneous IPC calls using the same thread
ec03a9639ab5 doc: Precision and typos
2b4348193551 doc: Where possible, remove links to ryanofsky/bitcoin/
286fe469c9c9 util: Add helpful error message when failing to execute file

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: a4f92969649018ca70f949a09148bccfeaecd99a
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Oct 7, 2025
@ryanofsky
Squashed 'src/ipc/libmultiprocess/' changes from 13424cf2ecc1..a4f929…
abcd4c4 
…696490

a4f929696490 Merge bitcoin-core/libmultiprocess#224: doc: fix typos
f4344ae87da0 Merge bitcoin-core/libmultiprocess#222: test, ci: Fix threadsanitizer errors in mptest
1434642b3804 doc: fix typos
73d22ba2e930 test: Fix tsan race in thread busy test
b74e1bba014d ci: Use tsan-instrumented cap'n proto in sanitizers job
c332774409ad test: Fix failing exception check in new thread busy test
ca3c05d56709 test: Use KJ_LOG instead of std::cout for logging
7eb1da120ab6 ci: Use tsan-instrumented libcxx in sanitizers job
ec86e4336e98 Merge bitcoin-core/libmultiprocess#220: Add log levels and advertise them to users via logging callback
515ce93ad349 Logging: Pass LogData struct to logging callback
213574ccc43d Logging: reclassify remaining log messages
e4de0412b430 Logging: Break out expensive log messages and classify them as Trace
408874a78fdc Logging: Use new logging macros
67b092d835cd Logging: Disable logging if messsage level is less than the requested level
d0a1ba7ebf21 Logging: add log levels to mirror Core's
463a8296d188 Logging: Disable moving or copying Logger
83a2e10c0b03 Logging: Add an EventLoop constructor to allow for user-specified log options
58cf47a7fc8c Merge bitcoin-core/libmultiprocess#221: test default PassField impl handles output parameters
db03a663f514 Merge bitcoin-core/libmultiprocess#214: Fix crash on simultaneous IPC calls using the same thread
afcc40b0f1e8 Merge bitcoin-core/libmultiprocess#213: util+doc: Clearer errors when attempting to run examples + polished docs
6db669628387 test In|Out parameter
29cf2ada75ea test default PassField impl handles output parameters
1238170f68e8 test: simultaneous IPC calls using same thread
eb069ab75d83 Fix crash on simultaneous IPC calls using the same thread
ec03a9639ab5 doc: Precision and typos
2b4348193551 doc: Where possible, remove links to ryanofsky/bitcoin/
286fe469c9c9 util: Add helpful error message when failing to execute file
47d79db8a552 Merge bitcoin-core/libmultiprocess#201: bug: fix mptest hang, ProxyClient<Thread> deadlock in disconnect handler
f15ae9c9b9fb Merge bitcoin-core/libmultiprocess#211: Add .gitignore
4a269b21b8c8 bug: fix ProxyClient<Thread> deadlock if disconnected as IPC call is returning
85df96482c49 Use try_emplace in SetThread instead of threads.find
ca9b380ea91a Use std::optional in ConnThreads to allow shortening locks
9b0799113557 doc: describe ThreadContext struct and synchronization requirements
d60db601ed9b proxy-io.h: add Waiter::m_mutex thread safety annotations
4e365b019a9f ci: Use -Wthread-safety not -Wthread-safety-analysis
15d7bafbb001 Add .gitignore
fe1cd8c76131 Merge bitcoin-core/libmultiprocess#208: ci: Test minimum cmake version in olddeps job
b713a0b7bfbc Merge bitcoin-core/libmultiprocess#207: ci: output CMake version in CI script
0f580397c913 ci: Test minimum cmake version in olddeps job
d603dcc0eef0 ci: output CMake version in CI script

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: a4f92969649018ca70f949a09148bccfeaecd99a
Sjors added a commit to Sjors/sv2-tp that referenced this pull request Oct 7, 2025
@Sjors
Squashed 'src/ipc/libmultiprocess/' changes from 47d79db8a5..a4f9296964
3e34afe 
a4f9296964 Merge bitcoin-core/libmultiprocess#224: doc: fix typos
f4344ae87d Merge bitcoin-core/libmultiprocess#222: test, ci: Fix threadsanitizer errors in mptest
1434642b38 doc: fix typos
73d22ba2e9 test: Fix tsan race in thread busy test
b74e1bba01 ci: Use tsan-instrumented cap'n proto in sanitizers job
c332774409 test: Fix failing exception check in new thread busy test
ca3c05d567 test: Use KJ_LOG instead of std::cout for logging
7eb1da120a ci: Use tsan-instrumented libcxx in sanitizers job
ec86e4336e Merge bitcoin-core/libmultiprocess#220: Add log levels and advertise them to users via logging callback
515ce93ad3 Logging: Pass LogData struct to logging callback
213574ccc4 Logging: reclassify remaining log messages
e4de0412b4 Logging: Break out expensive log messages and classify them as Trace
408874a78f Logging: Use new logging macros
67b092d835 Logging: Disable logging if messsage level is less than the requested level
d0a1ba7ebf Logging: add log levels to mirror Core's
463a8296d1 Logging: Disable moving or copying Logger
83a2e10c0b Logging: Add an EventLoop constructor to allow for user-specified log options
58cf47a7fc Merge bitcoin-core/libmultiprocess#221: test default PassField impl handles output parameters
db03a663f5 Merge bitcoin-core/libmultiprocess#214: Fix crash on simultaneous IPC calls using the same thread
afcc40b0f1 Merge bitcoin-core/libmultiprocess#213: util+doc: Clearer errors when attempting to run examples + polished docs
6db6696283 test In|Out parameter
29cf2ada75 test default PassField impl handles output parameters
1238170f68 test: simultaneous IPC calls using same thread
eb069ab75d Fix crash on simultaneous IPC calls using the same thread
ec03a9639a doc: Precision and typos
2b43481935 doc: Where possible, remove links to ryanofsky/bitcoin/
286fe469c9 util: Add helpful error message when failing to execute file

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: a4f92969649018ca70f949a09148bccfeaecd99a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanofsky ryanofsky ryanofsky approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Eunovo @DrahtBot @ryanofsky